nit

This is a bit confusing and dangerous. The variable bmoKustomization is used both for the old and upgraded BMO kustomization so it is easy to make a mistake when reasoning about it. Could you change it so we have separate variables?

Also, it seems strange to delete the old kustomization after the upgrade. At that point it should be the upgraded BMO we delete. If there is an issue and the test fails early, we should anyway stop. I don't think there is much point trying to clean up at that point.

I think the point here is to clean up so the next test can start, so no matter at what stage the test stops, the cluster will be restored to its original state. If the upgrade was done, then we would delete the upgraded deployment, but even in case, for some reason, it was skipped, then we still cleanup the upgrade-from deployment. Of course, it would mean we may do a bit extra in our cleanup, but imo that's acceptable, and shouldn't cause any harm (similarly, we always clean up both vbmc and sushy-tools containers, even though only one of them should exist at one point).

In the latest patch I renamed the param bmoKustomization in one place as you suggested. I also changed all reference of kustomize in function names to kustomization, for consistency.

Thanks the kustomization functions and variables look much better now! I don't think the defer makes sense in tests though. We should use the AfterEach instead, that is what it is for and where we clean up everything else from a test. This way we also make sure that we can collect relevant logs before we remove the containers.

If you really want to do the double deletion of both the old and upgraded I can accept that, but please note that it clutters the logs with a lot of errors and the test should anyway not continue if there was an error. If the upgrade from 0.4 fails for example then we stop and collect all data we can. No more tests should run at that point so cleaning up or not does not matter.

Thank you! My intention is to add ironic Upgrade into the same suite, so imo full cleanup, even doubling ones, is necessary.

I don't understand, however, why DeferCleanup is not suitable? It's listed as a way to clean up in Ginkgo docs, and compared to AfterEach, I think it is safer in this case: we only need to make sure to put one DeferCleanup after each installation, and we're fine. Otherwise, to put these cleanups into AfterEach, we would have to keep track of what was installed and what was not (for e.g. ironic is there, but it was preinstalled so shouldn't be cleaned, etc.)

I don't think missing logs is an issue either. DeferCleanup should be triggered only after the It block has ended. In our case it means the trigger phase will be similar to as if we put that cleanup in AfterEach. (In fact, it runs AFTER AfterEach, so it should be safe even if we want to put any extra log collection in AfterEach)

Ah I learned something new. Thanks for that! I was not aware of how this DeferCleanup worked. This looks good to me now :)